add: баг-репорт в игре, отправляется в дс#8484
add: баг-репорт в игре, отправляется в дс#8484kkadmor wants to merge 12 commits intoss220-space:master220from
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new in-game bug reporting feature that sends messages to a Discord webhook. While the feature is useful, the current implementation lacks critical security controls. Specifically, there is no rate limiting or mute checking on the player-accessible verb, which allows for easy spamming and potential Denial of Service of the Discord webhook. Additionally, the handling of user input allows for Discord mention injection, enabling users to ping arbitrary roles or users on the Discord server. Beyond these security concerns, a critical issue was found with an incorrect variable name in config/example/config.txt:387, which could prevent the feature from working correctly. Further recommendations include improving empty field checks, ensuring all input fields have appropriate length limits to prevent Discord API rejections, correcting a variable name typo, and removing a technical comment.
config/example/config.txt
Outdated
| ## DISCORD_BUGFIX_WEBHOOK_URLS https://url1 | ||
| ## DISCORD_BUGFIX_WEBHOOK_URLS https://url2 | ||
| #DISCORD_BUGFIX_WEBHOOK_URLS |
There was a problem hiding this comment.
🔴 Критично: Неверное имя переменной конфигурации в примере. В config/example/config.txt используется DISCORD_MENTOR_BUGREPORT_URLS, в то время как код ожидает DISCORD_BUGREPORT_WEBHOOK_URLS. Это приведет к тому, что функция не будет работать при использовании примера конфигурации.
## DISCORD_BUGREPORT_WEBHOOK_URLS https://url1
## DISCORD_BUGREPORT_WEBHOOK_URLS https://url2
#DISCORD_BUGREPORT_WEBHOOK_URLS
There was a problem hiding this comment.
буквально до того как выложил ПР поправил
| /client/verb/bugreport() | ||
| set name = "Баг-репорт" | ||
| set category = ADMIN_CATEGORY_TICKETS | ||
|
|
||
| var/msg | ||
|
|
||
| var/description = tgui_input_text(src, "Опишите баг/недочет:", "Баг-репорт", encode = FALSE) | ||
| var/correct_desc = tgui_input_text(src, "Опишите ожидаемое поведение (как должно работать):", "Баг-репорт", encode = FALSE) | ||
| var/discord = tgui_input_text(src, "Ваш дискорд для связи (обязательно):", "Баг-репорт", encode = FALSE) | ||
| var/have_sreens = tgui_alert(src, "Есть ли у вас скрины?", "Баг-репорт", list("Да", "Нет"))=="Да" | ||
|
|
||
| if(!description || !correct_desc || !discord) | ||
| tgui_alert(src, "Вы пропустили один из пунктов! Попробуйте сделать баг-репорт еще раз, заполняя все поля", "Баг-репорт") | ||
| return TRUE | ||
|
|
||
| msg = "[key_name(src)]\nRound id: [GLOB.round_id] \n1. [description]\n2. [correct_desc]\n3. [discord]\n4. Скрины: [have_sreens ? "Да" : "Нет"]" | ||
| if(tgui_alert(src, "Ваш репорт выглядит так:\n[msg]\nВы уверены что все заполнено правильно?", "Баг-репорт", list("Да", "Нет"))=="Да") | ||
| SSdiscord.send2discord_simple(DISCORD_WEBHOOK_BUGREPORT, msg) |
There was a problem hiding this comment.
The /client/verb/bugreport() verb lacks any form of rate limiting (cooldown) or mute checks. This allows malicious users to spam the Discord webhook, potentially leading to a Denial of Service or the webhook being deleted. Additionally, the input fields have no max_length specified, which could lead to Discord API rejections for overly long messages and potential resource exhaustion during JSON encoding. Remediation includes implementing a cooldown (e.g., 10 minutes per player), checking for player mutes (MUTE_ADMINHELP), and setting a reasonable max_length for all tgui_input_text calls (e.g., 500-1000 characters).
| if(!description || !correct_desc || !discord) | ||
| tgui_alert(src, "Вы пропустили один из пунктов! Попробуйте сделать баг-репорт еще раз, заполняя все поля", "Баг-репорт") | ||
| return TRUE |
There was a problem hiding this comment.
🟡 Рекомендация: Проверка на пустые поля неполная. Условие if(!description || !correct_desc || !discord) пропустит пустые строки, так как !"" вернет FALSE. Это может привести к отправке неполных баг-репортов. Рекомендуется использовать !length() для проверки, что поля действительно заполнены.
if(!length(description) || !length(correct_desc) || !length(discord))
tgui_alert(src, "Вы пропустили один из пунктов! Попробуйте сделать баг-репорт еще раз, заполняя все поля", "Баг-репорт")
return TRUE
There was a problem hiding this comment.
с каких пор !"" = FALSE?
| var/description = tgui_input_text(src, "Опишите баг/недочет:", "Баг-репорт", encode = FALSE) | ||
| var/correct_desc = tgui_input_text(src, "Опишите ожидаемое поведение (как должно работать):", "Баг-репорт", encode = FALSE) | ||
| var/discord = tgui_input_text(src, "Ваш дискорд для связи (обязательно):", "Баг-репорт", encode = FALSE) |
There was a problem hiding this comment.
The bug report system is vulnerable to Discord mention injection. The input is taken with encode = FALSE in tgui_input_text, and the Discord subsystem later calls html_decode on the content in code/datums/discord.dm. This combination allows users to inject Discord-specific mentions such as <@&role_id> or <@user_id> by either typing them directly or using HTML entities. This can be used to bypass the weak @everyone and @here filters to ping and harass Discord users or roles.
Remediation: Sanitize the input to remove or escape Discord mention syntax (e.g., by escaping < or @) before passing it to send2discord_simple.
| /// Webhook URLs for the requests webhook | ||
| /datum/config_entry/str_list/discord_requests_webhook_urls | ||
|
|
||
| //Needs attention |
| var/description = tgui_input_text(src, "Опишите баг/недочет:", "Баг-репорт", encode = FALSE) | ||
| var/correct_desc = tgui_input_text(src, "Опишите ожидаемое поведение (как должно работать):", "Баг-репорт", encode = FALSE) | ||
| var/discord = tgui_input_text(src, "Ваш дискорд для связи (обязательно):", "Баг-репорт", encode = FALSE) | ||
| var/have_sreens = tgui_alert(src, "Есть ли у вас скрины?", "Баг-репорт", list("Да", "Нет"))=="Да" |
There was a problem hiding this comment.
🟡 Рекомендация: Обнаружена опечатка в имени переменной have_sreens. Согласно стайлгайду (п. 4.1, Именование), имена переменных должны быть осмысленными. Для улучшения читаемости кода предлагаю переименовать ее в have_screens. Не забудьте также обновить использование этой переменной в строке 16.
var/have_screens = tgui_alert(src, "Есть ли у вас скрины?", "Баг-репорт", list("Да", "Нет"))=="Да"
References
- Имена переменных должны быть осмысленными и следовать стилю lower_snake_case. Имя
have_sreensсодержит опечатку. (link)
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new in-game bug reporting tool that sends reports to Discord via webhooks. While a useful feature, the current implementation has critical security flaws. The most significant is a bypass of Discord mention protections (like @everyone and user/role pings) using HTML entities, caused by raw input handling (encode = FALSE) and automatic HTML decoding in the Discord subsystem. This lack of proper input sanitization also opens up possibilities for malicious Markdown injection. Additionally, the verb lacks rate limiting, which could lead to webhook flooding. Minor issues include inconsistent use of return TRUE and mixed languages in Discord messages.
| var/description = tgui_input_text(src, "Опишите баг/недочет:", "Баг-репорт", max_length=700, encode = FALSE) | ||
| var/correct_desc = tgui_input_text(src, "Опишите ожидаемое поведение (как должно работать):", "Баг-репорт", max_length=700, encode = FALSE) | ||
| var/discord = tgui_input_text(src, "Ваш дискорд для связи (обязательно):", "Баг-репорт", max_length=100, encode = FALSE) |
There was a problem hiding this comment.
This section is critically vulnerable to Discord ping and mention bypasses. The use of encode = FALSE in tgui_input_text combined with the Discord subsystem's html_decode() on content allows attackers to use HTML entities (e.g., @everyone) to bypass mention protections. This also creates a risk of malicious Markdown injection and potential XSS if the input is ever rendered as HTML in-game. It's crucial to ensure proper encoding of user input to prevent these vulnerabilities.
var/description = tgui_input_text(src, "Опишите баг/недочет:", "Баг-репорт", max_length=700)
var/correct_desc = tgui_input_text(src, "Опишите ожидаемое поведение (как должно работать):", "Баг-репорт", max_length=700)
var/discord = tgui_input_text(src, "Ваш дискорд для связи (обязательно):", "Баг-репорт", max_length=100)
| return TRUE | ||
|
|
||
| msg = "[key_name(src)]\nRound id: [GLOB.round_id] \n1. [description]\n2. [correct_desc]\n3. [discord]\n4. Скрины: [have_screens ? "Да" : "Нет"]" | ||
| if(findtext(msg, "<@") || findtext(msg, "<&@")) |
There was a problem hiding this comment.
The Discord ping check is bypassable and contains a typo. Discord role pings are formatted as <@&ID>, but the code checks for <&@. Furthermore, because the input should be HTML encoded (see previous comment), the check should look for the encoded version <@ to be effective against the html_decode() performed in the Discord subsystem.
if(findtext(msg, "<@") || findtext(msg, "<@"))
| if(!description || !correct_desc || !discord) | ||
| tgui_alert(src, "Вы пропустили один из пунктов! Попробуйте сделать баг-репорт еще раз, заполняя все поля", "Баг-репорт") | ||
| return TRUE |
There was a problem hiding this comment.
This verb lacks proper spam prevention and rate limiting, unlike adminhelp. This omission could allow a malicious user to flood the Discord channel with reports. Implementing handle_spam_prevention() is crucial here. Additionally, for consistency, return should be used instead of return TRUE.
if(handle_spam_prevention(description + correct_desc + discord, MUTE_ADMINHELP, OOC_COOLDOWN))
return
| tgui_alert(src, "Вы пропустили один из пунктов! Попробуйте сделать баг-репорт еще раз, заполняя все поля", "Баг-репорт") | ||
| return TRUE | ||
|
|
||
| msg = "[key_name(src)]\nRound id: [GLOB.round_id] \n1. [description]\n2. [correct_desc]\n3. [discord]\n4. Скрины: [have_screens ? "Да" : "Нет"]" |
There was a problem hiding this comment.
🟡 Рекомендация по локализации и стилю
Сообщение, отправляемое в Discord, содержит смесь русского и английского языков (Round id и Скрины). Согласно стайлгайду, логи и сообщения для разработчиков должны быть на английском языке для единообразия. Рекомендуется полностью перевести строку на английский.
Также, для лучшей читаемости, предлагаю использовать Yes/No вместо Да/Нет.
msg = "[key_name(src)]\nRound id: [GLOB.round_id] \n1. [description]\n2. [correct_desc]\n3. [discord]\n4. Screenshots: [have_screens ? "Yes" : "No"]"
References
- Логи и другие сообщения, предназначенные для разработчиков, не должны переводиться и должны быть на английском языке. (link)
| msg = "[key_name(src)]\nRound id: [GLOB.round_id] \n1. [description]\n2. [correct_desc]\n3. [discord]\n4. Скрины: [have_screens ? "Да" : "Нет"]" | ||
| if(findtext(msg, "<@") || findtext(msg, "<&@")) | ||
| tgui_alert(src, "Обнаружена попытка пинга (<@id> или <@&id>)! Сообщение не будет отправленно.", "Баг-репорт") | ||
| return TRUE |
There was a problem hiding this comment.
🟡 Рекомендация по стилю
Аналогично предыдущему замечанию, для консистентности кода следует использовать return вместо return TRUE.
return
References
- Обеспечивать единый стиль для всей кодовой базы. (link)
|
<@305281601280147456> |
|
/gemini review |
There was a problem hiding this comment.
Code Review
Аналитический отчёт от Большого Брата. Обработка запроса завершена.
Приветствую! Я проанализировал предложенные Вами изменения. С технической точки зрения, работа выглядит требующей доработок. Вы добавили полезную функцию для отправки баг-репортов из игры, что упростит сбор обратной связи. Однако, есть несколько моментов, которые требуют внимания для повышения безопасности и качества кода.
Сводка анализа:
- 🔴 Критических замечаний: 1
- 🟡 Рекомендаций: 1
- 💡 Предложений: 0
🔴 Критические замечания (требуют обязательного исправления):
code/modules/admin/verbs/bugreport.dm:11-18: Отсутствует санитарная обработка пользовательского ввода, что создает потенциальную уязвимость. Также можно улучшить логику сбора данных.
🟡 Рекомендации по улучшению:
code/controllers/configuration/entries/config.dm:655: В коде присутствует временный комментарий//Needs attention, который следует удалить для поддержания чистоты кода.
Пожалуйста, ознакомьтесь с комментариями к отдельным строкам кода для получения более подробной информации.
Спасибо за ваш вклад в проект!
| var/description = tgui_input_text(src, "Опишите баг/недочет:", "Баг-репорт", max_length=700, encode = FALSE) | ||
| var/correct_desc = tgui_input_text(src, "Опишите ожидаемое поведение (как должно работать):", "Баг-репорт", max_length=700, encode = FALSE) | ||
| var/discord = tgui_input_text(src, "Ваш дискорд для связи (обязательно):", "Баг-репорт", max_length=100, encode = FALSE) | ||
| var/have_screens = tgui_alert(src, "Есть ли у вас скрины?", "Баг-репорт", list("Да", "Нет"))=="Да" | ||
|
|
||
| if(!description || !correct_desc || !discord) | ||
| tgui_alert(src, "Вы пропустили один из пунктов! Попробуйте сделать баг-репорт еще раз, заполняя все поля", "Баг-репорт") | ||
| return |
There was a problem hiding this comment.
🔴 Критично (потенциальная уязвимость и улучшение UX): Код в этом блоке имеет две проблемы:
- Уязвимость: Входные данные от пользователя (
description,correct_desc,discord) не проходят санитарную обработку. Это может привести к злоупотреблению форматированием в Discord. Рекомендуется использоватьsanitize_simple(). - UX: Все окна ввода появляются последовательно, даже если пользователь отменил предыдущий шаг. Лучше проверять ввод после каждого шага и прерывать процедуру при отмене. Это соответствует правилу о ранних возвратах (стайлгайд, п. 4.1, строка 74).
Предлагаю объединить исправления в одном изменении.
var/description = sanitize_simple(tgui_input_text(src, "Опишите баг/недочет:", "Баг-репорт", max_length=700, encode = FALSE))
if(!description) return
var/correct_desc = sanitize_simple(tgui_input_text(src, "Опишите ожидаемое поведение (как должно работать):", "Баг-репорт", max_length=700, encode = FALSE))
if(!correct_desc) return
var/discord = sanitize_simple(tgui_input_text(src, "Ваш дискорд для связи (обязательно):", "Баг-репорт", max_length=100, encode = FALSE))
if(!discord) return
var/have_screens = tgui_alert(src, "Есть ли у вас скрины?", "Баг-репорт", list("Да", "Нет"))=="Да"
References
- Предпочитать ранние возвраты (
return) глубокой вложенности условий, а такжеcontinueв циклах. (link)
There was a problem hiding this comment.
sanitize_simple() убирает \n и \t, скорее всего из-за траблов в игре. У вебхука это не вызывает никаких проблем, я протестил
| /// Webhook URLs for the requests webhook | ||
| /datum/config_entry/str_list/discord_requests_webhook_urls | ||
|
|
||
| //Needs attention |
There was a problem hiding this comment.
🟡 Рекомендация по стилю: Комментарий //Needs attention выглядит как временная пометка и не соответствует стайлгайду. Рекомендую его удалить, чтобы код оставался чистым и понятным. Согласно стайлгайду (п. 4.1, строка 68), комментарии должны быть на английском языке и объяснять, почему код написан так, а не что он делает.
References
- Комментарии должны быть на английском языке и объяснять, почему код написан так, а не что он делает. (link)
…правке багрепорта
| if(!discord) | ||
| empty_input_alert(3) | ||
| return | ||
| var/have_screens = tgui_alert(src, "4. Есть ли у вас скрины?", "Баг-репорт", list("Да", "Нет"))=="Да" |
There was a problem hiding this comment.
"4. Есть ли у вас скрины/видео?" почему бы и не видео 🤷#зюзяянетруп
|
Ожидает ревью и тд. |
|
Ожидай и тд |
2 similar comments
|
Ожидай и тд |
|
Ожидай и тд |
| if(findtext(msg, "<@") || findtext(msg, "<@&")) | ||
| tgui_alert(src, "Обнаружена попытка пинга (<@id> или <@&id>)! Сообщение не будет отправленно.", "Баг-репорт") | ||
| return | ||
| if(tgui_alert(src, "Ваш репорт выглядит так:\n[msg]\nВы уверены что все заполнено правильно?", "Баг-репорт", list("Да", "Нет"))=="Да") |
There was a problem hiding this comment.
| if(tgui_alert(src, "Ваш репорт выглядит так:\n[msg]\nВы уверены что все заполнено правильно?", "Баг-репорт", list("Да", "Нет"))=="Да") | |
| if(tgui_alert(src, "Ваш репорт выглядит так:\n[msg]\nВы уверены что все заполнено правильно?", "Баг-репорт", list("Да", "Нет")) == "Да") |
| if(handle_spam_prevention(msg, MUTE_ADMINHELP, OOC_COOLDOWN)) | ||
| return | ||
|
|
||
| if(tgui_alert(src, "Ваш репорт выглядит так:\n[msg]\nВы уверены что все заполнено правильно?", "Баг-репорт", list("Да", "Нет"))=="Да") |
There was a problem hiding this comment.
Сделай нормальный тгуи интерфейс. Нет нужды плодить кучу инпутов

Что этот ПР делает
Добавляет в панель тикетов кнопку "Баг-репорт" в которую можно сообщить о проблеме, которая отправится в дискорд разработчикам.
При использовании задает следующие вопросы:
Необходимо создать специальный канал в дискорде с доступом для разработчиков и создать вэбхук. В конфиге (config/config.txt) надо создать DISCORD_BUGFIX_WEBHOOK_URLS куда следует указать ссылку на созданный вэбхук.
!! Это не замена багрепортам из дискорда, а просто альтернативный способ их отправки
Почему это хорошо для игры
Игрокам часто лень заходить в дс и писать багрепорты по форме. Тут это подается в удобной форме прямо в игре и разработчики получают нужную информацию. Также разработчики могут связаться с игроком в дискорде чтобы попросить скрины или уточнить что-то
Демонстрация изменений
Демонстрации изменений
Если что-то упущенно:

Тестирование
локалка с вэбхуком на своем сервере